feat(issue-details): Add runtime label for JS issues #88312
Conversation
github-actions
bot
added
the
Scope: Frontend
s1gr1d
changed the title
s1gr1d
force-pushed
the
sig/runtime-label
branch
from
571ee2b to
021ad3f
Compare
There was a problem hiding this comment.
While i do think this solves a problem for nextjs etc. I'm not sure it makes sense for sentry.javascript.cloudflare or even our own frontend project. Should we check the project platform?
It also maybe needs a tooltip? the word "Frontend" by itself is maybe not the most clear
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #88312 +/- ##
==========================================
+ Coverage 87.70% 87.71% +0.01%
==========================================
Files 10143 10063 -80
Lines 573197 568665 -4532
Branches 22518 22353 -165
==========================================
- Hits 502729 498817 -3912
+ Misses 70036 69452 -584
+ Partials 432 396 -36 |
|
|
||
| if ( | ||
| (event.contexts.runtime && !event.contexts.browser) || | ||
| event.contexts?.cloud_resource // Cloudflare, Vercel etc. |
There was a problem hiding this comment.
| event.contexts?.cloud_resource // Cloudflare, Vercel etc. | |
| event.contexts.cloud_resource // Cloudflare, Vercel etc. |
only one optional chaining here, I guess either everywhere or nowhere?
| } | ||
|
|
||
| if ( | ||
| (event.contexts.runtime && !event.contexts.browser) || |
There was a problem hiding this comment.
I think that backend events also get a browser context, sadly...? I believe this is extracted from the user agent of the incoming request.
There was a problem hiding this comment.
Usually not, but I can exclude the browser check here 👍
There was a problem hiding this comment.
I think we can just check for if (event.contexts.runtime) { // this is backend } - this should be true for all current JS SDKs, I think:
- For bun:
runtime: { name: 'bun', version: Bun.version }, - For cloudflare:
runtime: { name: 'cloudflare' }, - For deno:
runtime: { name: 'deno', version: Deno.version.deno }, - For node:
runtime: { name: 'node', version: global.process.version }, - For vercel-edge:
runtime: { name: 'vercel-edge' },
We do not set this for any other SDKs, as far as I can tell.
Obviously this is not super robust, but should be good for the time being I guess..? We could additionally also check for contexts.cloud_resource I suppose!
alternatively we could also allow-list the values of runtime.name:
if (['node', 'bun', 'deno', 'cloudflare', 'vercel-edge'].includes(contexts.runtime?.name)) {
// backend
}but then we need a robust check for browser too 🤔 In the future, we could check on contexts.page which would be robust as this would be frontend-only. Today, we can only do weird heuristics I guess - e.g. contexts.browser + !contexts.runtime, for example...?
| */ | ||
| export function getRuntimeLabelAndTooltip( | ||
| event: Event, | ||
| knownRuntimeType?: {isBackend?: boolean; isFrontend?: boolean} |
There was a problem hiding this comment.
this is a bit of a weird API, what if you pass { isBackend: true, isFrontend: true }? I'd rather make this an enum:
knownRuntimeType?: 'frontend' | 'backend'
| (event.contexts.runtime?.name && | ||
| ['node', 'bun', 'deno', 'cloudflare', 'vercel-edge'].includes( | ||
| event.contexts.runtime.name |
There was a problem hiding this comment.
we can simplify this to:
| (event.contexts.runtime?.name && | |
| ['node', 'bun', 'deno', 'cloudflare', 'vercel-edge'].includes( | |
| event.contexts.runtime.name | |
| (['node', 'bun', 'deno', 'cloudflare', 'vercel-edge'].includes( | |
| event.contexts.runtime?.name || '' |
or something like this, I believe?
| event.contexts.runtime.name | ||
| )) | ||
| ) { | ||
| return {label: t('Backend'), tooltip: t('Error from Server, Edge or Worker Runtime')}; |
There was a problem hiding this comment.
actually, can we be more specific here - based on the runtime name? :D So if it is node / bun / deno, "Error from Server", if it is cloudflare "Error from Worker Runtime" if it is vercel-edge "Error from Edge", or something like this?
Thinking about this, maybe we can/should just remove the knownRuntimeType escape hatch and use this always, this would be kind of neat I guess... But no strong feelings!
Add "Frontend" and "Backend" to quickly see the runtime.   part of #85732
Add "Frontend" and "Backend" to quickly see the runtime.   part of #85732
Add "Frontend" and "Backend" to quickly see the runtime.
part of #85732